-
Notifications
You must be signed in to change notification settings - Fork 181
Fixes Register Endpoint for Router of Instance of APIWebSocketRoute #358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes Register Endpoint for Router of Instance of APIWebSocketRoute #358
Conversation
5d17034 to
bed8cfb
Compare
|
Hey @yuval9313, could you or another maintainer take a look in this merge request please? Thanks a lot, |
yuval9313
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are also missing test case
fastapi_utils/cbv.py
Outdated
| for route in router.routes: | ||
| if not isinstance(route, APIRoute): | ||
| raise ValueError("The provided routes should be of type APIRoute") | ||
| if not isinstance(route, APIRoute) and not isinstance(route, APIWebSocketRoute): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, this is really nit pick but I prefer not (condition a or condition b)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem at all, i'll happily make the change :)
b12a8cd to
a224089
Compare
Hello, I've added a test case in the unittests for the websocket example, it tests the three basic operations on the websocket connection (can accept connection, can send_text, receive_text and can close). I hope this test is okay :) |
790d46a to
5e2b161
Compare
|
Hello @yuval9313 , can we resume this MR? I'd love to have this merged |
Versions
Python Version: 3.13
Package Versions:
Bug
When creating a Route of type
APIWebSocketRoutethe following error is thrown:This error can be replicated using the following example:
Solution
The solution i recommend is to just add the class to instance check of the
_register_endpointsfunction and then add it with a fixedroute_methodof"WS"since theAPIWebSocketRoutedoes not have amethodsatribute.Thank you very much,
Vitor Hideyoshi.